Skip to content

Comments

feat: add key size validation#613

Merged
bshaffer merged 2 commits intomainfrom
key-size-validation
Dec 15, 2025
Merged

feat: add key size validation#613
bshaffer merged 2 commits intomainfrom
key-size-validation

Conversation

@bshaffer
Copy link
Collaborator

@bshaffer bshaffer commented Dec 3, 2025

fixes #605

Slightly different implementation for #609

@bshaffer bshaffer merged commit 6b80341 into main Dec 15, 2025
8 checks passed
@bshaffer bshaffer deleted the key-size-validation branch December 15, 2025 18:45
@ejunker
Copy link

ejunker commented Dec 16, 2025

thank you @bshaffer for getting this fix in. Does someone need to contact Mitre and get the CVE updated to show that there is a fixed version?

@ro0NL
Copy link

ro0NL commented Jan 7, 2026

@bshaffer should there be a way to opt-out? IIUC currently we need to request new secrets from trusted external partners on our side now :/

@bshaffer
Copy link
Collaborator Author

bshaffer commented Jan 7, 2026

It's protected in a major version of the library so the way to opt out is to not upgrade to the major version until the keys are of sufficient length.

I do recommend contacting your partners if their keys do not reach the minimum length security requirements, however

@ro0NL
Copy link

ro0NL commented Jan 7, 2026

fair; i was wondering how bad a short key really is in case of internal api calls, to a trusted partner. Unpractical at least :')

@bshaffer
Copy link
Collaborator Author

bshaffer commented Jan 7, 2026

@ro0NL I don't know, I just wanted the big red flags that said my library was insecure to go away

@ro0NL
Copy link

ro0NL commented Jan 8, 2026

our partner cannot generate new tokens, so we're stuck at v6, which is an issue.

@ro0NL
Copy link

ro0NL commented Jan 8, 2026

@bshaffer would you consider something like passing new InsecureKey($secret) for a key?

@bshaffer
Copy link
Collaborator Author

bshaffer commented Jan 8, 2026

@ro0NL feel free to submit a PR and I'm happy to review it!

I wouldn't expect you to have any problems staying on v6 for the near future. It also seems strange to me that a partner would not be able to generate new tokens. You could consider padding your existing tokens to reach the minimum key length.

@ro0NL
Copy link

ro0NL commented Jan 8, 2026

@bshaffer thanks for the hint about padding, this seems a reasonable workaround 👍

$secret = str_pad($secret, 32, "\0"); works as-is with v7 👼

@jannes-io
Copy link

It's protected in a major version of the library so the way to opt out is to not upgrade to the major version until the keys are of sufficient length.

@bshaffer but then we also need to opt out of composer block-insecure, which is also a manual interaction.

We're completely hard locked now,... we can't easily switch keys on all these projects, and they're all failing their auto-updates or automatic installation... This is quite ridicules for the type of vulnerability we've got here,.. it's not up to the library to decide what length of keys we're using??

@bshaffer
Copy link
Collaborator Author

but then we also need to opt out of composer block-insecure

I don't understand what this means, can you explain it? You should be able to just pin to ^6.0 of this library in composer.json. if the block-insecure is required because of the vulnerability, then that's the whole reason for the fix.

@jannes-io
Copy link

but then we also need to opt out of composer block-insecure

I don't understand what this means, can you explain it? You should be able to just pin to ^6.0 of this library in composer.json. if the block-insecure is required because of the vulnerability, then that's the whole reason for the fix.

We have an automated deployment system that automatically updates and creates new projects with a fresh composer.json.

When it starts, it now throws:

o composer.lock file present. Updating dependencies to latest instead of installing from lock file. See https://getcomposer.org/install for more information.
Loading composer repositories with package information
Updating dependencies
Your requirements could not be resolved to an installable set of packages.

  Problem 1
    - Root composer.json requires ourlibrary
    - ourlibrary require firebase/php-jwt ^6.8 -> found firebase/php-jwt[v6.8.0, ..., v6.11.1] but these were not loaded, because they are affected by security advisories. To ignore the advisories, add ("PKSA-y2cr-5h3j-g3ys") to the audit "ignore" config. To turn the feature off entirely, you can set "block-insecure" to false in your "audit" config.
 

So there is currently NO way to run composer install/update without any manual interactions. We can't "opt out" by keeping this library at 6.x, because that is blocked by composer. This change is a critical backwards compatibility break..

@ro0NL
Copy link

ro0NL commented Feb 20, 2026

you can ignore the specific CVE in composer.json audit config: "ignore": ["CVE-2025-45769"]

@jannes-io
Copy link

you can ignore the specific CVE in composer.json audit config: "ignore": ["CVE-2025-45769"]

@ro0NL That is NOT a backwards compatible change, if I need to do a manual interaction, it means it's not backwards compatible??

We can't have thousands of deployments suddenly break and require manual unblocking one by one because of a "possible security issue if you have a short key somewhere and use one of the affected algorithms". A warning? Sure. A deprecation notice? Perfect. Making packages completely uninstallable? C'mon guys what are we doing...

@ro0NL
Copy link

ro0NL commented Feb 20, 2026

i see, i thought it would be a few projects on your end to patch.

Im not really sure it qualifies a bc break, because CVE's are filed on existing versions. Technically the code didnt change :')

@ejunker
Copy link

ejunker commented Feb 20, 2026

We have an automated deployment system that automatically updates and creates new projects with a fresh composer.json.

@jannes-io So can't you pin firebase/php-jwt to ^6.0? Are you saying it always uses the latest versions of all of your dependencies? That seems like a problem. What if you are on PHP 8.4 and then one of your dependencies releases a new version that requires PHP 8.5 then that would also break your apps.

@jannes-io
Copy link

So can't you pin firebase/php-jwt to ^6.0? Are you saying it always uses the latest versions of all of your dependencies? That seems like a problem.

@ejunker If I hadn't pinned php-jwt to ^6.0 then we wouldn't be having this conversation.

After wasting our time on investigating our usage of php-jwt, we discovered we could upgrade to 7.0 without issue, and so we did and released it. I suppose yet another thing to opt-out from (composer audit blocking) has been added to our default setup as well to prevent it in the future...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

⚠️ php-jwt < v6.11.0 was discovered to contain weak encryption.

5 participants